This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Hmmm my apologies, but I think I might need some extra time for a final look at this before merging 🙏

Thanks, that makes sense.

My intent with the current implementation was to:

• preserve the existing UnknownComponent behavior for the single-component case
• aggregate unknown component names only when multiple invalid components are present
• keep the rest of the behavior as close as possible to the existing flow

One of the motivations for introducing UnknownComponents was also to avoid repeating the full error message multiple times when several invalid components are provided, which felt a bit noisy UX-wise. The grouped format seemed more concise and easier to read in that case.

That said, I agree the mixed behavior could be confusing if multiple different error types occur. I’m happy to adjust the approach once the intended UX for #4794 is confirmed with @FranciscoTGouveia whether that means aggregating only UnknownComponent* and bailing early for others, or reshaping the error handling more uniformly.

Also, point on UnknownComponent vs UnknownComponents: I introduced the second mainly to avoid regressing the existing single-component behavior, but I’m open to simplifying that if we settle on a cleaner model.

And agreed on the tuple, a small struct would likely make that clearer if we keep the grouped variant.

The original intention of 4794 was indeed to install all possible components and report back every invalid one.

For example, running rustup component add rust-analyzere rustc-dev llvm-toolse, should install rustc-dev successfully, while returning two separate errors, one for each misspelled component.

On another note, I briefly tested the patch and believe it would be worth adding a test case covering the example above, as this is the current output:

$ my-rustup component add rust-analyzere rustc-dev llvm-toolse
error: toolchain '1.90-x86_64-unknown-linux-gnu' does not contain component 'rust-analyzere' for target 'x86_64-unknown-linux-gnu'; did you mean 'rust-analyzer'?

The original intention of 4794 was indeed to install all possible components and report back every invalid one.

@FranciscoTGouveia Thanks for your reply! Given that, my suggestions would be:

• It'd be easier to just aggregate all UnknownComponent* errors in the installation process and bail out immediately like before if other kinds of errors do occur.

• On the distinction of UnknownComponent vs UnknownComponents, I'd say it's more idiomatic to special-case the display format on a unified UnknownComponents, rather than doing two enum variants. In other words: if the Vec is of length 1 we print like the old UnknownComponent, otherwise we use the new printing format as introduced here.

• It'd be better if the helper struct could be introduced.

@Inconnu08 What do you think?

(Assigned @FranciscoTGouveia for review upon his request.)

Apologies for the late response. Thanks to both of you for clarifying the intended behavior :)

My understanding now is:

• we should process all valid components: best-effort
• aggregate all UnknownComponent* errors and report them together
• still bail out immediately for other kinds of errors

Using a single UnknownComponents variant with display logic that special-cases the length (to preserve the current single-component/best-match UX) makes sense to me and simplifies the current approach.

The earlier split between UnknownComponent and UnknownComponents was mainly to avoid regressing the single-component formatting/suggestion behavior, but I’m happy to unify it now that the intended direction is clear.

I’ll update the implementation accordingly, and I’ll also:

• add a test covering the example (rust-analyzere rustc-dev llvm-toolse)
• extend the existing tests with a case where a valid component appears between invalid ones
• adjust the error formatting to move suggestions onto a new line for readability

Let me know if that aligns before I push the changes.

Thanks again @Kobzol , your newly-enabled automations have made the tracking of subtle changes across rebases much easier 🙏

@Inconnu08 Great work!

There are some test-related stuff that I'll update for you :)

Thanks again @Kobzol , your newly-enabled automations have made the tracking of subtle changes across rebases much easier 🙏

That was actually implemented by @Urgau :)